Skip to content

feat: 1172 refresh feedsearch view asynchronically#1294

Merged
qcdyx merged 70 commits intomainfrom
1172-refresh-feedsearch-view-asynchronically
Aug 6, 2025
Merged

feat: 1172 refresh feedsearch view asynchronically#1294
qcdyx merged 70 commits intomainfrom
1172-refresh-feedsearch-view-asynchronically

Conversation

@qcdyx
Copy link
Copy Markdown
Contributor

@qcdyx qcdyx commented Jul 21, 2025

Summary:

Closes #1172

created a refresh materialized view task executor that refresh feedsearch view asynchronously
bounced the refresh task timestamp to the next :00 or :30 to deduplicate multiple refreshes at the same time

Expected behavior:

image

Testing tips:

Provide tips, procedures and sample files on how to test the feature.
Testers are invited to follow the tips AND to try anything they deem relevant outside the bounds of the testing tips.

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with ./scripts/api-tests.sh to make sure you didn't break anything
  • Add or update any needed documentation to the repo
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

@qcdyx qcdyx requested a review from davidgamez July 28, 2025 02:26
Comment thread api/src/shared/database/database.py Outdated
Comment thread api/src/shared/database/database.py Outdated
Comment thread infra/functions-python/main.tf Outdated
name = "refresh-materialized-view-task-queue"

rate_limits {
max_concurrent_dispatches = 2
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's set this to 1. It shouldn't be more than one at the time

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread infra/functions-python/main.tf Outdated
Comment thread infra/functions-python/main.tf Outdated
Comment thread infra/functions-python/main.tf Outdated
PROJECT_ID = var.project_id
ENV = var.environment
PUBSUB_TOPIC_NAME = "rebuild-bounding-boxes-topic"
GOOGLE_FUNCTION_SOURCE = "src/main.py"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change necessary? I don't see a src folder in the deployed function.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed.

Comment thread functions-python/tasks_executor/src/main.py Outdated
Comment thread api/src/shared/database/database.py Outdated

# Enqueue the task
try:
client.create_task(request={"parent": parent, "task": task})
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion]: Consider using the shared function, create_http_task. The shared function takes care of the authentication token.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, will move the shared Logic into api/src/ and do a code refactoring due to direct imports not feasible (Docker/environment separation).

from shared.database.database import with_db_session, refresh_materialized_view
from shared.database.database import (
with_db_session,
create_refresh_materialized_view_task,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to add google-cloud-tasks dependency to the requirements.txt file of this function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

@qcdyx qcdyx force-pushed the 1172-refresh-feedsearch-view-asynchronically branch 3 times, most recently from e0a1035 to 474516b Compare August 6, 2025 00:10
@qcdyx qcdyx force-pushed the 1172-refresh-feedsearch-view-asynchronically branch from 474516b to 18d92ab Compare August 6, 2025 00:30
@qcdyx qcdyx requested a review from davidgamez August 6, 2025 11:31
Copy link
Copy Markdown
Member

@davidgamez davidgamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice refactoring! 💪

@qcdyx qcdyx merged commit 159bc76 into main Aug 6, 2025
8 of 9 checks passed
@qcdyx qcdyx deleted the 1172-refresh-feedsearch-view-asynchronically branch August 6, 2025 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refresh FeedSearch view asynchronically

2 participants